Skip to content

feat: Introduce v3 strict column definitions and enhanced type safety #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dpanta94 merged 35 commits into from
Sep 24, 2025
Merged

feat: Introduce v3 strict column definitions and enhanced type safety #43

dpanta94 merged 35 commits into from
Sep 24, 2025

Conversation

@dpanta94
Copy link
Member

@dpanta94 dpanta94 commented Sep 21, 2025 •
edited
Loading

Summary

  • Introduces breaking changes for Schema v3 with strict column and index definitions
  • Implements comprehensive type safety with PHP type mappings
  • Adds robust query methods with automatic type transformations

API Updates for Table definition

Before

class MyTable extends Table {
			const SCHEMA_VERSION = '2.0.0';
			protected static $base_table_name = 'simple';
			protected static $group = 'bork';
			protected static $schema_slug = 'bork-simple';
			protected static $uid_column = 'id';

			protected function get_definition() {
				global $wpdb;
				$table_name      = self::table_name( true );
				$charset_collate = $wpdb->get_charset_collate();

				return "
					CREATE TABLE `{$table_name}` (
						`id` int(11) UNSIGNED NOT NULL AUTO_INCREMENT,
						`name` varchar(25) NOT NULL,
						`slug` varchar(25) NOT NULL,
						`something` varchar(25) NOT NULL,
						PRIMARY KEY (`id`),
						KEY `slug` (`slug`),
						KEY `something` (`something`)
					) {$charset_collate};
				";
                        }

After

<?php
class MyTable extends Table {
	const SCHEMA_VERSION = '2.0.0';
	protected static $base_table_name = 'simple';
	protected static $group = 'bork';
	protected static $schema_slug = 'bork-simple';

	public static function get_version_1_0_0_schema(): Table_Schema {
		$table_name = static::table_name();
		$columns = new Column_Collection();

		$columns[] = ( new ID( 'id' ) )->set_length( 11 )->set_type( Column::COLUMN_TYPE_INT );
		$columns[] = ( new String_Column( 'name' ) )->set_length( 25 );
		$columns[] = ( new String_Column( 'slug' ) )->set_length( 255 );

		return new Table_Schema( $table_name, $columns );
	}

	public static function get_version_1_2_0_schema(): Table_Schema {
		$table_name = static::table_name();
		$columns = new Column_Collection();

		$columns[] = ( new ID( 'id' ) )->set_length( 11 )->set_type( Column::COLUMN_TYPE_INT );
		$columns[] = ( new String_Column( 'name' ) )->set_length( 25 );
		$columns[] = ( new String_Column( 'slug' ) )->set_length( 25 )->set_is_index( true );

		return new Table_Schema( $table_name, $columns );
	}

	public static function get_version_2_0_0_schema(): Table_Schema {
		$table_name = static::table_name();
		$columns = new Column_Collection();

		$columns[] = ( new ID( 'id' ) )->set_length( 11 )->set_type( Column::COLUMN_TYPE_INT );
		$columns[] = ( new String_Column( 'name' ) )->set_length( 25 );
		$columns[] = ( new String_Column( 'slug' ) )->set_length( 25 )->set_is_index( true );
		$columns[] = ( new String_Column( 'something' ) )->set_length( 25 )->set_is_index( true );

		return new Table_Schema( $table_name, $columns );
	}

	public static function get_schema_history(): array {
		return [
			static::SCHEMA_VERSION => [self::class, 'get_version_2_0_0_schema'],
			'1.2.0' => [self::class, 'get_version_1_2_0_schema'],
			'1.0.0' => [self::class, 'get_version_1_0_0_schema'],
		];
	}

	public static function transform_from_array( array $result_array ) {
		return $result_array;
	}
}

Note: The method get_definition has changed from protected to public and its part of the interface. The Abstract Table implementation provides a way to translate your Table Schema in a dbDelta compatible SQL definition. You can always still overwrite that method if you choose so and provide your own definition.

Note: The history approach, even though currently is NOT being utilized, it allows to move towards a migration system (similar to Laravel's).

The API protects you from some common mistakes with MySQL 5.5. Like multiple timestamp columns defaulting on CURRENT_TIMESTAMP or having a varchar indexed be longer than 191 chars

@dpanta94 dpanta94 self-assigned this Sep 21, 2025
@lucatume
Copy link
Contributor

lucatume commented Sep 22, 2025

  1. Why the change of name from "Field" to "Column"?
  2. To make the PR description easier to read, I would show a before/after example of a table definition changed. Currently one has to dig to tests/wpunit/Tables/ComplexTableTest.php to see the new API in action.
  3. With the aim of this being a new major version, the breaking change of stopping support for a dbDelta compatible SQL definition could make sense, but I cannot find, in the tests, an example of a destructive update (one where a column is removed in a later version of the schema); what would happen in that case?

@dpanta94
Copy link
Member Author

dpanta94 commented Sep 22, 2025

  1. This was a choice that i communicated with @borkweb through a loom (maybe not clear enough? im waiting for his feedback). But also in writting: I felt that the Field naming as a whole wasn't clear enough that it was supposed to act as the columns of the table. Renaming them to Columns i feel makes it crystal clear. I also believe that the Field system had a level of control and a complex enough API that would provide more than (usually at least) needed. For example in TEC we didnt utilize it. The new and possible inline Column_Collection definition, for me is superior in the ways below:

  • The API forces you to use it, while also guides you on how to use it. If you know how to write a column definition in SQL you will figure out how to write a column definition using that API in less than a minute assuming that you are using a good IDE.

  • The API DOES NOT require you creating new implementations of fields/columns in order to use them. You can just do inline $collection[] = new ID( 'id'); which helps you avoid creating code you dont need (and then maintaining it)

  • Before v3, each table's version would evaluate each subfields version too. That was ok since each Field would be a different implementation (new file). Still as a developer my expectations would be that if i changed a column definition i would have to bump the table's schema version. I think managing multiple versions added to the complexity.

  1. I have updated the PR description to amend for that.
  2. We didn't stop support for dbDelta compatible definitions. We are still dbDelta compatible or at least this is the desired outcome. The tests so far show we are. The provided Table Abstract has the get_definition implemented and its aim is to build the dbDelta compatible SQL definition through the user provided Table Schema definition. That means that you can always choose to overwrite the get_definition method and return your own hardcoded SQL definition.

Additionally, more test coverage is present that is shown in the diff. The reason is that i updated the underlying code that was already powering the existing test suites. The specific test case you are asking is called should_update_table_when_version_changes. It's not part of the diff, but it exists runs and passes.

lucatume
lucatume requested changes Sep 22, 2025
View reviewed changes
Copy link
Contributor

@lucatume lucatume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea and the new method-based API, smaller changes.

@dpanta94 dpanta94 mentioned this pull request Sep 22, 2025
Closed
@dpanta94 dpanta94 requested a review from lucatume September 23, 2025 16:26
borkweb
borkweb reviewed Sep 23, 2025
View reviewed changes
Copy link
Contributor

@borkweb borkweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this a lot and I agree that the change from Field to Column is clearer. Nice work!

I didn't review with a fine-toothed comb, so I won't mark it as Approved, but the direction and structure feels really good.

lucatume
lucatume approved these changes Sep 24, 2025
View reviewed changes
Copy link
Contributor

@lucatume lucatume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work.
We've discussed the implementation in a huddle going over the code.

@dpanta94 dpanta94 merged commit af298fc into main Sep 24, 2025
2 checks passed
@dpanta94 dpanta94 deleted the feat/v3-strict-column-definitions branch September 24, 2025 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucatume lucatume lucatume approved these changes

@Camwyn Camwyn Awaiting requested review from Camwyn

+1 more reviewer

@borkweb borkweb borkweb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@dpanta94 dpanta94

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dpanta94 @lucatume @borkweb